Skip to content

Make Func::in / Func::clone_in transitive#9117

Open
abadams wants to merge 2 commits intomainfrom
abadams/transitive_clone_in
Open

Make Func::in / Func::clone_in transitive#9117
abadams wants to merge 2 commits intomainfrom
abadams/transitive_clone_in

Conversation

@abadams
Copy link
Copy Markdown
Member

@abadams abadams commented May 1, 2026

When the Func passed to in()/clone_in() does not directly call the wrapped Func, walk down the call graph and wrap the first direct caller found along each branch. This makes it possible to schedule wrappers across anonymous intermediates (e.g. helper Funcs returned from free-standing C++ functions like the downsamplers in local_laplacian), without needing a handle on every Func in between.

Funcs that already directly call the target pass through unchanged, and Funcs with no static path to the target are left alone so existing sequences of in()/clone_in() that rely on later substitution still work.

Includes a new correctness test (transitive_in) and uses the new feature in apps/local_laplacian to clone gPyramid[0] into gPyramid[1] (~10% speedup).

This has been a long-standing annoyance that I'm finally fixing. Feature implemented by claude. I just did the change to local_laplacian to use it.

When the Func passed to in()/clone_in() does not directly call the
wrapped Func, walk down the call graph and wrap the first direct caller
found along each branch. This makes it possible to schedule wrappers
across anonymous intermediates (e.g. helper Funcs returned from
free-standing C++ functions like the downsamplers in local_laplacian),
without needing a handle on every Func in between.

Funcs that already directly call the target pass through unchanged, and
Funcs with no static path to the target are left alone so existing
sequences of in()/clone_in() that rely on later substitution still work.

Includes a new correctness test (transitive_in) and uses the new feature
in apps/local_laplacian to clone gPyramid[0] into gPyramid[1] (~10%
speedup).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@alexreinking alexreinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good feature! Lets you treat intermediate/inlined Funcs a bit more like Exprs. I'd suggest adding an example to the Func Doxygen comments, though.

@abadams
Copy link
Copy Markdown
Member Author

abadams commented May 1, 2026

I should add there's a design decision here that I went back and forth on: For the Funcs between *this and the args, do you clone them too? I.e. are we cloning the whole subgraph, or are we just walking back to the Func and cloning that one? I ended up not cloning the whole subgraph because I worried about cases where some of the intermediates are named. If you implicitly clone one of the intermediates without realizing it, and then schedule the original, it will only schedule the other uses of it. Seems like a foot-gun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants